Contextualize credentials used by GitSCMFileSystem when possible
#1802
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeromepochat
reviewed
Sep 3, 2025
Comment on lines
+384
to
+386
| if (_build != null && credential != null && credential.forRun(_build) instanceof StandardCredentials standardCredential) { | ||
| credential = standardCredential; | ||
| } |
There was a problem hiding this comment.
question: No sure if GitSCMTelescope should contextualize the credentials similarly?
Member
Author
There was a problem hiding this comment.
Yeah I looked into that, but IDK. There is also this credentials lookup in AbstractGitSCMSource which won't work, and I don't think it can be made to work:
git-plugin/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java
Lines 1333 to 1338 in de7f436
Going by https://github.com/search?type=code&q=+owner%3Ajenkinsci+gitscmtelescope, I think the answer here would just be that
GitSCMSource doesn't support inference-based options for GitHubAppCredentials, you must use GitHubSCMSource. IDK if there would ever be a reason that you would have to use GitSCMSource over GitHubSCMSource, but I don't think so.
For GitSCM and its use of GitSCMFileSystem, the situation is different, since there is no GitHubSCM.
jeromepochat
reviewed
Sep 3, 2025
| } | ||
|
|
||
| @Test | ||
| public void filesystem_supports_credential_contextualization() throws Exception { |
There was a problem hiding this comment.
note: I confirm that the test fails without the credentials contextualization.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After jenkinsci/github-branch-source-plugin#822, credential lookups for
GitHubAppCredentialsconfigured to use an inference-based repository access strategy fail when usingGitSCMFileSystem. NormallyGitHubSCMFileSystemis used instead ofGitSCMFileSystem, and that works fine, butGitSCMFileSystemgets used in cases where you have to configure aGitSCMdirectly, since there is no GitHub-specific SCM implementation.For example, you can run into this problem if you use "Pipeline script from SCM" to configure a Pipeline, enable lightweight checkout, and use
GitHubAppCredentialsfor theGitSCMcredentials.For more context, credential lookups for
GitHubAppCredentialswere expected to fall into one of two cases:Connector.lookupScanCredentials. This ensures proper owner inference and bypasses repository inference because the credential usage context is trusted. Plugins that need to do this kind of lookup may require changes like Use Connector.lookupScanCredentials to contextualize GitHubAppCredentials github-checks-plugin#398 (but others were already usingConnector.lookupScanCredentials).withCredentialsstep. Plugins that need to do this kind of lookup need to useCredentialsProvider.findCredentialByIdand pass an appropriateRuncontext for proper owner and repository inference, like is already done in this plugin inGitSCM.lookupScanCredentials.GitSCMFileSystemis a bit of an awkward spot. Conceptually, it falls under case 1 and should useConnector.lookupScanCredentialsto contextualizeGitHubAppCredentialsfor a trusted context, but we can't add agithub-branch-sourcedependency here or else we'll have circular dependencies. This leaves us with two options:credentialsthat is comparable toCredentials.forRunthat allows contextualization of generic credential lookups, and avoids the need to useConnector.lookupScanCredentialsdirectly when working withGitHubAppCredentialsGitHubSCMFileSystemwithGitSCM, or a GitHub-specific SCM implementation or similar that bypassesGitSCMFileSystemin this caseGitSCMFileSystem, although repository inference strategies would still not be supportedCC @jeromepochat
Testing done
See new automated test.
Submitter checklist